-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Workstealing take 3 #58500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Workstealing take 3 #58500
Conversation
|
|
||
| mutable struct Node{T} | ||
| const value::Union{T, Nothing} | ||
| @atomic next::Union{Node{T}, Nothing} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can keep this type stable by having next point to itself in case there is no next object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd just replace the small union get_next(node) === nothing with a get_next(node) === node check everywhere though. Not sure it'd help performance, and it'd likely cause some annoying footguns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intent could at least be made clearer with a helper function hasnext(node) = get_next(node) != node.
| # Weak Memory Models | ||
| # ======= | ||
|
|
||
| module CLL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you update this with my bugfixes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I did. the x86 specific stuff was part of them but I also didn't test CLL too much (just that it built)
This just forward ports the workstealing PR to be on top of #56475 though that PR is a lot more mergeable than this one.